This repository has been archived by the owner on Jun 26, 2023. It is now read-only.
sync pinner on every pin operation #13
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
petar
changed the title
aschmahmann
suggested changes
Jun 29, 2021
|
@aschmahmann Take a look to see if this logging strategy looks sufficient to narrow down the issue. Added flushes to all public mutable methods. |
gammazero
added a commit
that referenced
this pull request
Jul 19, 2021
This PR builds on PR #13 to sync the pinner on every pin operation. This PR does the following that #13 does not: - Sync's dag service separately from pin data - Does not release and immediately reacquire lock to sync, syncs while still holding pinner lock. - Syncs only pin data for most operations In addition to sync of pin data, this PR also revises how indexes are rebuilt. Instead of reading through all pins and loading all previous index data, now only a single pass through the pins is needed to rebuild missing indexes resulting from incomplete add or delete operations. This is operationally much simpler, but also does not require storing entire pin sets or index sets in memory as did the previous solution.
Merged
|
Subsumed by #15. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Addresses ipfs/kubo#8149 by syncing after every Pin.
The benchmark below compares sync-on-pin vs sync-on-batch for Badge and LevelDB.
It shows that the impact of syncing on every pin operation is not significant.
BenchmarkSyncOnceBadger-10 1694 599379 ns/op
BenchmarkSyncEveryBadger-10 1406 859930 ns/op
BenchmarkSyncOnceLevelDB-10 18 59809635 ns/op
BenchmarkSyncEveryLevelDB-10 12 104517039 ns/op
We deployed the patched pinner in production on (a) collabs and (b) nft clusters.
(a) Ran for a few hours. Collab servers do not perform much if any pinning. Ran for a few hours. After restart, the server took 22mins to come online, no reindexing was triggered. FYI here, the init system that handles starting/stopping ipfs in prod sends a forceful kill to the process; if it doesn't die within a certain time of it getting the clean termination signal. This should have no relation to pinner dirty/reindexing.